Skip to content

fix: preserve Unicode when clipping file lines - #1960

Merged
edelauna merged 1 commit into
Zoo-Code-Org:mainfrom
WebMad:fix/pr1950-unicode-clipping
Oct 9, 2026
Merged

edelauna merged 1 commit into
Zoo-Code-Org:mainfrom
WebMad:fix/pr1950-unicode-clipping

Conversation

@WebMad

@WebMad WebMad commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Related GitHub Issue

Part of #1948. Standalone Unicode-clipping bugfix extracted from #1950; this does not close the issue or replace the reader-foundation PR.

Description

Preserve supplementary Unicode characters when existing file readers clip a long line. The previous boundary could retain only a high surrogate, so UTF-8 encoding introduced a replacement character and corrupted emoji.

  • Change only the clipping block in formatWithLineNumbers(): reserve the existing ellipsis, then move the boundary back one UTF-16 code unit if the last retained unit is a high surrogate.
  • Add indentation-reader-unicode.spec.ts with 28 outcome-based tests covering the default long-line regression, both surrogate-range endpoints, complete/crossing/following pairs, BMP text, combining marks, ZWJ sequences, tiny limits, ASCII, aligned line numbers, and both reader modes' content/error/metadata contracts.
  • Keep existing imports, constants, signatures, numeric behavior, selection logic, and result types unchanged. No dependencies, settings, reader classes, approval/cancellation changes, or unrelated refactoring. This is surrogate-pair safety, not grapheme segmentation or normalization.

The branch starts at the current upstream main, 2baac5e, independently of the reader refactor. The production hunk is identical to the one proposed for extraction from #1950; only the isolated regression suite was expanded for the requested verification. After this fix is merged by maintainers, #1950 can update its base and drop the duplicate clipping hunk and original Unicode regression.

Verification

Local Node 22.23.1, pnpm 10.8.1, frozen lockfile; all final checks below passed:

  • Regression before applying the fix: 61 passed, 9 failed, 70 total. All nine failures were surrogate-clipping scenarios; the existing reader suite passed. Two additional tiny-limit boundary tests were added afterward.
  • Final focused suites: 72 passed (28 new Unicode tests + 44 existing reader tests).
  • Full backend Vitest run: 9,963 passed, 39 skipped, 0 failed, across 524 test files. No existing tests were weakened or newly skipped.
  • Workspace lint and type checks: 11 successful tasks each.
  • Scoped ESLint with suppression pruning: passed; suppression entries did not increase. The pruning command's serialization-only rewrite was restored to keep the two-file scope.
  • Scoped Prettier and whitespace diff checks: passed.
  • Extension bundle and required dependency/webview builds: 4 successful tasks. Commit hooks also passed.

Reproduction uses the scripts in package.json, src/package.json, and the existing backend Vitest configuration:

pnpm lint
pnpm check-types
pnpm exec turbo run bundle --filter=zoo-code
cd src
npx vitest run --maxWorkers=2 --coverage \
  --coverage.include='integrations/misc/indentation-reader.ts' \
  --coverage.reporter=text --coverage.reporter=json \
  --coverage.reporter=json-summary --coverage.reporter=lcov \
  --coverage.reportsDirectory=coverage/unicode-full

Coverage evidence and scope

The existing V8 provider instruments the entire reader module, with no working-code exclusions or ignored lines. Function-region results below are derived from that full-module Istanbul report; the function's callback is included.

Scope Lines Functions Branches Statements
Complete extracted formatter, including all changed production code 100% (11/11) 100% (2/2) 100% (11/11) 100% (13/13)
Entire reader module, including untouched code 82.06% (119/145) 85.71% (12/14) 84% (84/100) 81.25% (130/160)

The whole-module number is not claimed to be 100%. Its uncovered private header helper and non-contiguous-range code predate this change and are outside the extraction. Those remain visible in the raw coverage output rather than being deleted, excluded, or exercised through artificial instrumentation.

The existing lockfile pairs Vitest 4.1.11 with coverage-v8 4.1.9 and prints a mixed-version warning. Coverage generation and all checks completed successfully; this PR does not silently change dependencies. Prettier also warns about a pre-existing unknown ignore option. These warnings are reported rather than suppressed.

Documentation Updates

No user-facing documentation changes are needed: existing file reading now preserves complete surrogate pairs at the clipping boundary, without a new API or option.

Additional Notes

Implemented and verified with Zoo assistance. Keep this PR unmerged; automated and human review proceed through the repository's managed workflow. Approval will only be reported after an explicit, current-head CodeRabbit APPROVED review, not from silence or an informational comment.

Additional CI evidence

Changed-code mutation testing completed successfully on the published head. The downloadable report contains 14 mutants in the changed clipping lines, all Killed, with no survivors. This verifies the arithmetic, surrogate endpoints and conjunction, boundary decrement, slicing, and ellipsis against outcome assertions rather than mere execution. CI compilation, CodeQL, dependency review, packaging, and Linux tests have also passed; remaining checks and automated review are tracked by GitHub rather than claimed complete here.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Zoo-Code-Org/Zoo-Code/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: acf28394-4c45-457a-abad-72e0e0e13d3d
📥 Commits

Reviewing files that changed from the base of the PR and between 2baac5e and d5cce91.

📒 Files selected for processing (2)
  • src/integrations/misc/__tests__/indentation-reader-unicode.spec.ts
  • src/integrations/misc/indentation-reader.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (4)
Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases.

⚙️ CodeRabbit configuration file

Files:

  • src/integrations/misc/__tests__/indentation-reader-unicode.spec.ts
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.

⚙️ CodeRabbit configuration file

Files:

  • src/integrations/misc/indentation-reader.ts
  • src/integrations/misc/__tests__/indentation-reader-unicode.spec.ts
Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.

⚙️ CodeRabbit configuration file

Files:

  • src/integrations/misc/indentation-reader.ts
  • src/integrations/misc/__tests__/indentation-reader-unicode.spec.ts
Act as an adversarial second-opinion reviewer.

⚙️ CodeRabbit configuration file

Files:

  • src/integrations/misc/indentation-reader.ts
  • src/integrations/misc/__tests__/indentation-reader-unicode.spec.ts

📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Prevented clipped text from splitting Unicode surrogate pairs, preserving complete characters in formatted output.
  • Tests

    • Added coverage for Unicode clipping and formatting, including emoji, combining characters, line numbering, limits, and reader responses.

Walkthrough

Long-line formatting now avoids ending clipped text on the high surrogate of a UTF-16 pair. New tests cover Unicode clipping and indentation-reader output, including ranges, metadata, empty input, and errors.

Changes

Unicode-safe indentation clipping

Layer / File(s) Summary
Surrogate-safe line clipping
src/integrations/misc/indentation-reader.ts, src/integrations/misc/__tests__/indentation-reader-unicode.spec.ts
Formatting backs up one code unit before the ellipsis when the truncation endpoint follows a high surrogate. Tests cover Unicode clipping boundaries, short lines, small limits, ASCII alignment, and empty formatting.
Indentation-reader clipping tests
src/integrations/misc/__tests__/indentation-reader-unicode.spec.ts
Reader tests check clipped Unicode output, ranges, metadata, default limits, empty input, and error responses.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to d5cce

The Unicode clipping change appears ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Regression Evidence ✅ Passed PASS. The changed clipping behavior is covered directly at the formatter layer by outcome-based tests for a surrogate pair at the boundary, complete and following pairs, surrogate-range endpoints, BMP…
Security Boundaries ✅ Passed PASS. The only production change is in formatWithLineNumbers() and adjusts the truncation endpoint when it would retain a UTF-16 high surrogate. It only formats existing file content and does not ad…
Persistence Integrity ✅ Passed The pull request changes only Unicode clipping in formatWithLineNumbers() and adds tests. The changed module reads and formats strings; it introduces no file writes, storage operations, transactions…
Lifecycle Resource Cleanup ✅ Passed No changed lifecycle path exists. The production change only adjusts the synchronous formatWithLineNumbers() string-clipping boundary before substring() and appends .... The added tests call syn…
Title check ✅ Passed The title clearly and concisely describes the main change: preserving Unicode characters when clipping file lines.
Description check ✅ Passed The description is detailed and covers the issue reference, implementation, scope, verification results, coverage, documentation impact, and additional notes. It does not reproduce the checklist or us…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: The required review sequence passed. Remaining merge requirements apply.

Review-state labels are managed by this workflow; do not edit them manually. community-approved is managed the same way — do not add or remove it manually. It signals a fresh community code approval for the current head as an advisory priority only; maintainer review is still required.

@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Oct 8, 2026
@github-actions github-actions Bot added awaiting-maintainer CodeRabbit approved; waiting for a human maintainer and removed coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Oct 8, 2026
WebMad added a commit to WebMad/Zoo-Code that referenced this pull request Oct 8, 2026
…#1948)

Build on the extracted Unicode clipping, shared image MIME guard, and streamed path fixes from PRs Zoo-Code-Org#1960, Zoo-Code-Org#1961, and Zoo-Code-Org#1962. Preserve their latest regression coverage while composing reader strategies, descriptor-bound access, approval, cancellation, and validated text/document results.
WebMad added a commit to WebMad/Zoo-Code that referenced this pull request Oct 8, 2026
…#1948)

Build on the extracted Unicode clipping, shared image MIME guard, and streamed path fixes from PRs Zoo-Code-Org#1960, Zoo-Code-Org#1961, and Zoo-Code-Org#1962. Preserve their latest regression coverage while composing reader strategies, descriptor-bound access, approval, cancellation, and validated text/document results.
WebMad added a commit to WebMad/Zoo-Code that referenced this pull request Oct 8, 2026
…#1948)

Build on the extracted Unicode clipping, shared image MIME guard, and streamed path fixes from PRs Zoo-Code-Org#1960, Zoo-Code-Org#1961, and Zoo-Code-Org#1962. Preserve their latest regression coverage while composing reader strategies, descriptor-bound access, approval, cancellation, and validated text/document results.

@edelauna edelauna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - thanks.

@edelauna
edelauna added this pull request to the merge queue Oct 9, 2026
@github-actions github-actions Bot removed the awaiting-maintainer CodeRabbit approved; waiting for a human maintainer label Oct 9, 2026
Merged via the queue into Zoo-Code-Org:main with commit a101c61 Oct 9, 2026
18 checks passed
nh2 added a commit to nh2/Zoo-Code that referenced this pull request Oct 10, 2026
…dows

This makes content past 2000 characters on a single line readable at all. It
previously had no address any parameter could express, leaving command execution
as the only way to reach it.

`read_file` truncated every line at 2000 characters, and content past that point
was completely unreachable. Not merely inconvenient, but unreachable: no
combination of parameters could return it.

The reason is that `read_file`'s `offset` and `limit` select *which lines* to
return. Neither has ever addressed a position *within* a line, and there was no
`column_offset` or equivalent. So for a 5000-character line, the agent could ask
for "line 2", receive the first 1997 characters followed by a bare `...`, and
then had no way to ask for anything further. The remaining 3000 characters had
no address the tool could express.

The practical effect: any file whose lines are long was partly invisible.
Single-line JSON, minified JS and CSS, CSV exports, HTML email bodies, long log
lines. The agent's only recourse was `execute_command`, shelling out to `sed`,
`cut` or `tail`, which requires command approval, pollutes the transcript, and
fails on Windows where those tools are usually absent. Any text editor or `less`
displays this content without difficulty; the agent could not.

Worse, the `...` marker said nothing about *how much* was missing, so the agent
could not even tell it was looking at a partial line worth investigating.

Add a third mode to `read_file`, alongside the existing `slice` (read lines
N..M) and `indentation` (read the code block around line N):

    read_file { path, mode: "bytes_as_utf8", bytes_as_utf8: { offset, limit } }

This reads a window of raw bytes at a byte offset and decodes it as UTF-8,
giving every position in a file an address regardless of where line breaks fall.
It reads with a positioned `fs.open`/`read` instead of loading the file, so a
file much larger than the window, or larger than memory, can still be paged
through a window at a time.

The window lives in its own `bytes_as_utf8: { offset, limit }` object rather
than reusing the top-level `offset`/`limit`, which remain 1-based *line* numbers
and are documented as ignored in this mode. Silently changing their unit based
on `mode` is how a caller ends up reading byte 1500 when it meant line 1500.

Truncation markers now state exactly what was dropped and how to get it,
replacing the bare `...`:

      2 | {"payload":"AAAA...[+1026 bytes omitted, starting at byte offset 5000
    up to and including byte offset 6025]

    Note: 1 line exceeded the 2000-byte per-line display cap (longest: 6026
    bytes). The omitted bytes have no line-based address.
    To read the omitted part of line 2: read_file with mode='bytes_as_utf8' and
    bytes_as_utf8.offset=5000.

Both endpoints are named, and both are inclusive, so acting on them needs no
arithmetic: the start offset goes directly into `bytes_as_utf8.offset`, and the
end offset says how large the remainder is without a probing read first. The
summary block repeats this once below the content, because a marker at the end
of a 2000-character line is easy to scroll past.

Because the output is *not* a faithful representation of arbitrary bytes: any
byte that is not valid UTF-8 is replaced with U+FFFD. Calling the mode `bytes`
would invite an agent to trust it for binary data it silently corrupts. The
explicit name also keeps `bytes` free for a future mode that shows bytes
verbatim, for example as hex.

**Byte offsets must be measured on the raw bytes, not on decoded text.** When
invalid bytes are decoded to U+FFFD and that text is re-encoded, each such byte
becomes three, so every offset after it shifts. Offsets derived from decoded
text would be wrong on precisely the malformed machine-generated files this
feature exists to read, and would send the follow-up byte read to the wrong
place. The file's `Buffer` is therefore passed through the read path, and line
boundaries are located by scanning it for 0x0A (via the native `Buffer.indexOf`)
instead of splitting a string.

**The per-line cap is now measured in bytes rather than characters**, because
that is the only way its cut point can be reported as a byte offset the new mode
accepts. An arbitrary byte position can fall inside a multi-byte character, so
the cut backs off to the nearest character boundary; otherwise the last visible
"character" would be a corrupt replacement glyph. Relatedly, the full 2000-byte
budget now goes to content with the marker appended outside it. Previously three
characters were reserved for `...`, so only 1997 were kept, three fewer than
documented.

Upstream Zoo-Code-Org#1960 ("fix(read-file): preserve Unicode when clipping file lines")
fixed the old character-based cut splitting a UTF-16 surrogate pair, by dropping
one more code unit when the last one kept was a high surrogate. This commit
replaces that cut, so the fix is superseded rather than kept alongside:

* Cutting at a UTF-8 sequence boundary never splits a code point, so it cannot
  split a surrogate pair either; the separate check has nothing left to catch.
* The byte-based cut point has a byte offset the marker can name. A character
  count cannot be turned into one reliably: on a file containing invalid UTF-8,
  offsets measured from the decoded text drift (see above).
* Upstream still ends the line with a bare `...`, which reads the same as a
  literal `...` in the file and says nothing about how much is missing.

The cost: the cap now counts bytes, so non-ASCII lines show fewer characters
before the cut. A line of emoji shows 500 instead of 998, CJK text about 666
characters. Neither version keeps grapheme clusters together; both only
guarantee that no code point is broken.

Upstream's `indentation-reader-unicode.spec.ts` is ported to the byte-based API
here, keeping its cases (sequences before, on and after the cap, the code points
around the surrogate range, combining marks, ZWJ sequences, and the slice and
indentation readers) with expectations re-derived for UTF-8 byte boundaries.

* Byte mode runs before the binary-file check and before any whole-file read.
  Both would defeat the point: the mode exists for files too large to hold or
  too unstructured for line reading, and an explicit byte offset is an
  unambiguous request for those bytes whatever the file's detected type.
* Reading at or past end-of-file returns an empty window rather than an error,
  so an agent paging by following reported offsets needs no special case for the
  final read.
* A multi-byte character split by the *window* edge is trimmed and reported,
  rather than decoded into a U+FFFD the agent could not distinguish from a
  replacement character genuinely present in the file. At end-of-file such a
  sequence is kept instead, since there it is real malformed data rather than an
  artefact of where the window stopped.
* Invalid bytes inside a window are counted and reported, for the same reason:
  so unexpected glyphs can be attributed to the file rather than to the read.
* The approval prompt shown to the user names bytes rather than lines, and omits
  an end offset when no limit was given, since the file may be shorter than the
  default window.

Existing `slice` and `indentation` behaviour is unchanged apart from the improved
truncation marker.

Assisted-by: Claude Opus 5 with Zoo Code
WebMad added a commit to WebMad/Zoo-Code that referenced this pull request Oct 10, 2026
…#1948)

Build on the extracted Unicode clipping, shared image MIME guard, and streamed path fixes from PRs Zoo-Code-Org#1960, Zoo-Code-Org#1961, and Zoo-Code-Org#1962. Preserve their latest regression coverage while composing reader strategies, descriptor-bound access, approval, cancellation, and validated text/document results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants